feat: increase text and interactive elems contrast#2621
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR introduces a foreground theme feature enabling users to select text colour contrast levels (muted, standard, contrast). Design system tokens are extended with a ChangesForeground Theme Feature
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/Settings/FgThemePicker.vue`:
- Around line 8-11: The selector construction using
settings.preferredForegroundTheme (variable id) can throw if id contains invalid
characters; update the prehydrate logic in FgThemePicker.vue to validate or
sanitize id before passing it to el.querySelector: either check that id exists
in the canonical list of theme ids used by this component (e.g., the array/prop
that holds available theme ids) and only build the selector when it matches, or
call CSS.escape(id) to safely escape the value before interpolation; ensure you
reference and use the same identifier names (settings.preferredForegroundTheme /
id and the el.querySelector(...) call) when applying the validation/sanitization
so querySelector cannot be given a malformed selector.
In `@app/utils/prehydrate.ts`:
- Around line 42-46: preferredForegroundTheme is read from settings and applied
directly to document.documentElement.dataset.fgTheme; validate it first against
the allowed set ["muted","standard","contrast"] before assigning. Update the
logic around settings.preferredForegroundTheme in prehydrate.ts to: check that
preferredForegroundTheme is a string and is one of the allowed ids (e.g., via a
small const ALLOWED_FG_THEMES = new Set([...]) or array.includes), and only then
set document.documentElement.dataset.fgTheme = preferredForegroundTheme;
otherwise skip the assignment (or fall back to a safe default).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6954aee-32cc-45e6-ac0c-22c64796aaad
📒 Files selected for processing (21)
app/assets/main.cssapp/components/AppHeader.vueapp/components/Button/Base.vueapp/components/Input/Base.vueapp/components/Select/Base.vueapp/components/Settings/FgThemePicker.vueapp/composables/useCommandPaletteGlobalCommands.tsapp/composables/useSettings.tsapp/pages/settings.vueapp/types/command-palette.tsapp/utils/prehydrate.tsi18n/locales/en.jsoni18n/schema.jsonlunaria/styles.tsshared/utils/constants.tstest/e2e/hydration.spec.tstest/nuxt/a11y.spec.tstest/nuxt/components/CommandPalette.spec.tstest/nuxt/components/HeaderConnectorModal.spec.tstest/nuxt/composables/use-command-palette-commands.spec.tsuno.theme.ts
| const id = settings.preferredForegroundTheme | ||
| if (id) { | ||
| const input = el.querySelector<HTMLInputElement>(`input[value="${id}"]`) | ||
| if (input) { |
There was a problem hiding this comment.
Harden selector construction against invalid persisted values.
Line 10 interpolates id from localStorage into a CSS selector. If that value is malformed, querySelector(...) can throw and abort this prehydrate callback. Validate id against known theme ids (or use CSS.escape(id)) before building the selector.
As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/Settings/FgThemePicker.vue` around lines 8 - 11, The selector
construction using settings.preferredForegroundTheme (variable id) can throw if
id contains invalid characters; update the prehydrate logic in FgThemePicker.vue
to validate or sanitize id before passing it to el.querySelector: either check
that id exists in the canonical list of theme ids used by this component (e.g.,
the array/prop that holds available theme ids) and only build the selector when
it matches, or call CSS.escape(id) to safely escape the value before
interpolation; ensure you reference and use the same identifier names
(settings.preferredForegroundTheme / id and the el.querySelector(...) call) when
applying the validation/sanitization so querySelector cannot be given a
malformed selector.
| // Apply foreground accent | ||
| const preferredForegroundTheme = settings.preferredForegroundTheme | ||
| if (preferredForegroundTheme) { | ||
| document.documentElement.dataset.fgTheme = preferredForegroundTheme | ||
| } |
There was a problem hiding this comment.
Validate preferredForegroundTheme before applying it to data-fg-theme.
Line 43 reads an untyped runtime value from localStorage and Line 45 applies it directly. Please guard it against the allowed ids (muted, standard, contrast) before setting document.documentElement.dataset.fgTheme, to keep prehydrate state deterministic and avoid invalid theme attributes.
As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/utils/prehydrate.ts` around lines 42 - 46, preferredForegroundTheme is
read from settings and applied directly to
document.documentElement.dataset.fgTheme; validate it first against the allowed
set ["muted","standard","contrast"] before assigning. Update the logic around
settings.preferredForegroundTheme in prehydrate.ts to: check that
preferredForegroundTheme is a string and is one of the allowed ids (e.g., via a
small const ALLOWED_FG_THEMES = new Set([...]) or array.includes), and only then
set document.documentElement.dataset.fgTheme = preferredForegroundTheme;
otherwise skip the assignment (or fall back to a safe default).
1 participant
🧭 Context
One of the issues with the a11y and a recurring topic with the community is poor text visibility. I've also experienced this myself. So, I'm trying to increase contrast. As usual, I did it quite carefully - changed the colors by 10% and added a new shade for the borders (I use it for inputs/selects and in the midtone for buttons [I think they need a redesign to make them more visible, not the borders])